Skip to content

Conversation

@xehpuk
Copy link
Contributor

@xehpuk xehpuk commented Apr 6, 2018

animateScroll previously returned immediately which resulted in updateHistory and afterAnimate to run before the animation. It nor returns a Promise which is resolved when the animation ends and is rejected if there isn't an element with the given ID.
This previously was a warning emitted by fbjs (and its last usage). So now it's kicked out of the readme.

The package-lock.json should be committed into source repositories.

@bySabi
Copy link
Collaborator

bySabi commented Apr 6, 2018

@xehpuk your commit are absolutely great.
Make animateScroll async was a good catch, I forget the non-blocking nature of setTimeout

Only one thing. Could you reimplement animateScroll with async/await? . I thing this will make the code more readable. If you have the time, of course :-)

@bySabi bySabi merged commit 95b03af into some-react-components:master Apr 6, 2018
@xehpuk
Copy link
Contributor Author

xehpuk commented Apr 6, 2018

Phew, not sure how to do it or if it's even possible without using a Promise for the "callback nature" of setTimeout. Maybe you have an idea?

@bySabi
Copy link
Collaborator

bySabi commented Apr 6, 2018

At the end, the solution, promisify setTimeout, looks more tricky.

Thanks for take your time on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants